Clean up syntax for supported Python versions. by alok · Pull Request #1963 · ray-project/ray

alok · 2018-04-28T08:08:29Z

What do these changes do?

Replaces syntax with better syntax.

The dict/set/tuple literals (curly braces) for comprehensions and instantiation is
clearer and faster than writing unidiomatic calls to each of those functions.

Related issue number

No.

Ran code through [pyupgrade](https://github.com/asottile/pyupgrade). This is supported in every Python version 2.7+.

No need to specify 0,1.. if paramters are passed in order.

AmplabJenkins · 2018-04-28T09:09:44Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/5103/
Test PASSed.

robertnishihara · 2018-04-29T05:39:00Z

python/ray/cloudpickle/cloudpickle.py

                out_names = set()
            else:
-                out_names = set(names[oparg]
-                                for op, oparg in _walk_global_ops(co))


Let's not make any changes to this file as it is directly copied from the cloudpickle repository.

Good point. Put in a PR w/cloudpickle to change that though.

robertnishihara · 2018-04-29T05:41:38Z

python/ray/dataframe/dataframe.py

        result = self._repr_helper_()
        final_result = repr(result).rsplit("\n\n", maxsplit=1)[0] + \
-            "\n\n[{0} rows x {1} columns]".format(len(self.index),
+            "\n\n[{} rows x {} columns]".format(len(self.index),


If we do this change (and others) we'll need to fix the indentation of the next line

I'm fine leaving this as is then, though I'd prefer that there be a git hook to require all code to be run through auto-formatters. It'd unify things like the use of certain syntax, indent, and quote styles.

robertnishihara · 2018-04-29T05:42:33Z

@devin-petersohn can you take a look at the changes to the dataframes files and see if these make sense to you? If you prefer we can make these changes only for non-dataframes files.

devin-petersohn · 2018-04-29T08:29:20Z

@robertnishihara @alok These changes are fine for the dataframes files. Thanks!

This reverts commit efa5ec8.

Drop use of set literal until cloudpickle uses it.

alok · 2018-04-30T05:51:08Z

I think those are both of the necessary changes.

AmplabJenkins · 2018-04-30T06:55:54Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/5127/
Test PASSed.

robertnishihara · 2018-05-01T00:57:03Z

Looks good, thanks! There are still a couple linting errors at https://travis-ci.org/ray-project/ray/jobs/372891975. Could you fix those?

We need to set up a git pre-push hook to automatically run this stuff.

alok · 2018-05-01T19:09:55Z

I think that's all the lint errors taken care of.

robertnishihara · 2018-05-01T19:55:48Z

Thanks @alok, yapf seems to be complaining (though unfortunately it doesn't list the diff).

$ .travis/yapf.sh
~/build/ray-project/ray/test ~/build/ray-project/ray
~/build/ray-project/ray
~/build/ray-project/ray/python ~/build/ray-project/ray
~/build/ray-project/ray
Reformatted staged files. Please review and stage the changes.
Files updated:
  python/ray/worker.py
  test/actor_test.py
  test/stress_tests.py

alok · 2018-05-01T20:20:57Z

@robertnishihara I ran yapf locally with the same flags and it gave no changes. Travis says the build ran 19 hours ago, so maybe it hasn't picked up the changes yet?

This should probably be merged after ray-project#1963.

pcmoritz · 2018-05-02T02:07:55Z

There are still linting errors, can you have another look?

robertnishihara · 2018-05-02T02:09:52Z

Maybe it's a version issue with yapf?

alok · 2018-05-03T01:44:13Z

The cloudpickle changes were just merged in, so we can pick them up next time we update those files.

alok · 2018-05-03T01:57:54Z

@robertnishihara

Closing and reopening a PR apparently forces Travis to rebuild, and the lint test is green now.

pcmoritz · 2018-05-03T12:27:58Z

Jenkins test this please

AmplabJenkins · 2018-05-03T13:32:07Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/5171/
Test PASSed.

* magic-methods: fmt Fix IndentationError Write magic methods for SampleBatch/PartialRollout Clean up syntax for supported Python versions. (ray-project#1963) [DataFrame] Implements mode, to_datetime, and get_dummies (ray-project#1956) [DataFrame] Fix dtypes (ray-project#1930) keep_dims -> keepdims (ray-project#1980) add pthread linking (ray-project#1986) [DataFrame] Add layer of abstraction to allow OID instantiation (ray-project#1984) [DataFrame] Fix blocking issue on _IndexMetadata passing (ray-project#1965)

* master: (21 commits) Expand local_dir in Trial init (ray-project#2013) Fixing ascii error for Python2 (ray-project#2009) [DataFrame] Implements df.update (ray-project#1997) [DataFrame] Implements df.as_matrix (ray-project#2001) [DataFrame] Implement quantile (ray-project#1992) [DataFrame] Impement sort_values and sort_index (ray-project#1977) [DataFrame] Implement rank (ray-project#1991) [DataFrame] Implemented prod, product, added test suite (ray-project#1994) [DataFrame] Implemented __setitem__, select_dtypes, and astype (ray-project#1941) [DataFrame] Implement diff (ray-project#1996) [DataFrame] Implemented nunique, skew (ray-project#1995) [DataFrame] Implements filter and dropna (ray-project#1959) [DataFrame] Implements df.pipe (ray-project#1999) [DataFrame] Apply() for Lists and Dicts (ray-project#1973) Clean up syntax for supported Python versions. (ray-project#1963) [DataFrame] Implements mode, to_datetime, and get_dummies (ray-project#1956) [DataFrame] Fix dtypes (ray-project#1930) keep_dims -> keepdims (ray-project#1980) add pthread linking (ray-project#1986) [DataFrame] Add layer of abstraction to allow OID instantiation (ray-project#1984) ...

* master: (25 commits) [DataFrame] Add direct pandas imports for MVP (ray-project#1960) Make ActorHandles pickleable, also make proper ActorHandle and ActorC… (ray-project#2007) Expand local_dir in Trial init (ray-project#2013) Fixing ascii error for Python2 (ray-project#2009) [DataFrame] Implements df.update (ray-project#1997) [DataFrame] Implements df.as_matrix (ray-project#2001) [DataFrame] Implement quantile (ray-project#1992) [DataFrame] Impement sort_values and sort_index (ray-project#1977) [DataFrame] Implement rank (ray-project#1991) [DataFrame] Implemented prod, product, added test suite (ray-project#1994) [DataFrame] Implemented __setitem__, select_dtypes, and astype (ray-project#1941) [DataFrame] Implement diff (ray-project#1996) [DataFrame] Implemented nunique, skew (ray-project#1995) [DataFrame] Implements filter and dropna (ray-project#1959) [DataFrame] Implements df.pipe (ray-project#1999) [DataFrame] Apply() for Lists and Dicts (ray-project#1973) Clean up syntax for supported Python versions. (ray-project#1963) [DataFrame] Implements mode, to_datetime, and get_dummies (ray-project#1956) [DataFrame] Fix dtypes (ray-project#1930) keep_dims -> keepdims (ray-project#1980) ...

* Add flake8 to Travis * Add flake8-comprehensions [flake8 plugin](https://github.com/adamchainz/flake8-comprehensions) that checks for useless constructions. * Use generators instead of lists where appropriate A lot of the builtins can take in generators instead of lists. This commit applies `flake8-comprehensions` to find them. * Fix lint error * Fix some string formatting The rest can be fixed in another PR * Fix compound literals syntax This should probably be merged after #1963. * dict() -> {} * Use dict literal syntax dict(...) -> {...} * Rewrite nested dicts * Fix hanging indent * Add missing import * Add missing quote * fmt * Add missing whitespace * rm duplicate pip install This is already installed in another file. * Fix indent * move `merge_dicts` into utils * Bring up to date with `master` * Add automatic syntax upgrade * rm pyupgrade In case users want to still use it on their own, the upgrade-syn.sh script was left in the `.travis` dir.

alok added 2 commits April 28, 2018 01:04

Use set/dict literal syntax

c801000

Ran code through [pyupgrade](https://github.com/asottile/pyupgrade). This is supported in every Python version 2.7+.

Drop unnecessary string format specification

efa5ec8

No need to specify 0,1.. if paramters are passed in order.

robertnishihara reviewed Apr 29, 2018

View reviewed changes

alok added 2 commits April 29, 2018 22:48

Revert "Drop unnecessary string format specification"

f9fd906

This reverts commit efa5ec8.

Undo changes to cloudpickle

62fbd28

Drop use of set literal until cloudpickle uses it.

Reformat code with YAPF

0b8f3f6

We need to set up a git pre-push hook to automatically run this stuff.

alok added a commit to alok/ray that referenced this pull request May 1, 2018

Fix compound literals syntax

9e3cf43

This should probably be merged after ray-project#1963.

alok closed this May 3, 2018

alok reopened this May 3, 2018

pcmoritz approved these changes May 3, 2018

View reviewed changes

pcmoritz merged commit cdf94c1 into ray-project:master May 3, 2018

alok mentioned this pull request May 6, 2018

Use flake8-comprehensions #1976

Merged

alok deleted the upgrade-syn branch May 21, 2018 10:03

Conversation

alok commented Apr 28, 2018

What do these changes do?

Related issue number

Uh oh!

AmplabJenkins commented Apr 28, 2018

Uh oh!

robertnishihara Apr 29, 2018

Choose a reason for hiding this comment

Uh oh!

alok Apr 30, 2018

Choose a reason for hiding this comment

Uh oh!

robertnishihara Apr 29, 2018

Choose a reason for hiding this comment

Uh oh!

alok Apr 30, 2018

Choose a reason for hiding this comment

Uh oh!

robertnishihara commented Apr 29, 2018

Uh oh!

devin-petersohn commented Apr 29, 2018

Uh oh!

alok commented Apr 30, 2018

Uh oh!

AmplabJenkins commented Apr 30, 2018

Uh oh!

robertnishihara commented May 1, 2018

Uh oh!

alok commented May 1, 2018

Uh oh!

robertnishihara commented May 1, 2018

Uh oh!

alok commented May 1, 2018

Uh oh!

pcmoritz commented May 2, 2018

Uh oh!

robertnishihara commented May 2, 2018

Uh oh!

alok commented May 3, 2018

Uh oh!

alok commented May 3, 2018

Uh oh!

pcmoritz commented May 3, 2018

Uh oh!

AmplabJenkins commented May 3, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants